-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add scripts for desk lifter control and calibration #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Python scripts for controlling a desk lifter via Raspberry Pi GPIO pins, including test scripts, calibration utilities, and height positioning functionality.
Key Changes:
- Added GPIO control scripts for desk up/down movement
- Implemented calibration-based height positioning logic
- Documented calibration measurements and movement rates
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| scripts/test_down.py | Test script for verifying DOWN button control via GPIO pin 27 |
| scripts/reset_to_lowest.py | Utility to reset desk to lowest position using 56-second calibrated duration |
| scripts/move_to_height.py | Main positioning script that resets to lowest then moves to target height based on calibration |
| scripts/lifter_calibration.txt | Documents measured calibration data (height range and movement rates) |
| Highest height: 54.5 inches | ||
|
|
||
| Down movement rate: 0.55 inches/second (measured from 54.5" to 49" in 10s) | ||
| Up movement rate: 0.55 inches/second (measured from 23.7" to 29.1" in 10s) |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent calibration data documentation: Line 7 states "Up movement rate: 0.55 inches/second" but move_to_height.py uses UP_RATE = 0.54. Additionally, line 6 states "Down movement rate: 0.55 inches/second" but this constant is not defined in move_to_height.py where it's used (line 34 hardcodes 56.0 seconds). Ensure calibration values are consistent across all files.
| Up movement rate: 0.55 inches/second (measured from 23.7" to 29.1" in 10s) | |
| Up movement rate: 0.54 inches/second (measured from 23.7" to 29.1" in 10s) |
scripts/move_to_height.py
Outdated
| if __name__ == "__main__": | ||
| try: | ||
| target = float(input(f"Enter target height in inches ({LOWEST_HEIGHT}-{HIGHEST_HEIGHT}): ")) | ||
| move_to_height(target) | ||
| finally: | ||
| release_up() | ||
| release_down() | ||
| GPIO.cleanup() |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script violates custom guideline against if __name__ == "__main__" patterns. According to the project's coding guidelines, this should be avoided unless explicitly requested. The code should be left as a top-level script instead.
scripts/move_to_height.py
Outdated
| # Calibration data | ||
| LOWEST_HEIGHT = 23.7 # inches | ||
| HIGHEST_HEIGHT = 54.5 # inches | ||
| UP_RATE = 0.54 # inches per second (from calibration) |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent calibration data: The calibration file shows Up movement rate: 0.55 inches/second but this script uses UP_RATE = 0.54. This mismatch will cause incorrect positioning calculations.
| UP_RATE = 0.54 # inches per second (from calibration) | |
| UP_RATE = 0.55 # inches per second (from calibration) |
scripts/move_to_height.py
Outdated
|
|
||
| def move_to_height(target_height): | ||
| if not (LOWEST_HEIGHT <= target_height <= HIGHEST_HEIGHT): | ||
| raise ValueError(f"Target height {target_height}'' is out of range.") |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message incorrectly displays the target height using double-quote notation ({target_height}'') instead of inch notation ({target_height}"). The double single-quote is confusing and non-standard.
| raise ValueError(f"Target height {target_height}'' is out of range.") | |
| raise ValueError(f'Target height {target_height}" is out of range.') |
scripts/move_to_height.py
Outdated
| raise ValueError(f"Target height {target_height}'' is out of range.") | ||
| # Step 1: Reset to lowest | ||
| print(f"Resetting to lowest position ({LOWEST_HEIGHT})...") | ||
| release_down() | ||
| input("Press Enter to move all the way DOWN (about 56 seconds from max height)...") | ||
| press_down() | ||
| time.sleep(56.0) | ||
| release_down() | ||
| print("At lowest position.") | ||
| # Step 2: Move up for calculated time | ||
| delta = target_height - LOWEST_HEIGHT | ||
| up_time = delta / UP_RATE | ||
| print(f"Moving UP for {up_time:.1f} seconds to reach {target_height}''...") | ||
| release_up() | ||
| input("Press Enter to move UP...") | ||
| press_up() | ||
| time.sleep(up_time) | ||
| release_up() | ||
| print(f"Arrived at {target_height}'' (approximate).") | ||
|
|
||
| if __name__ == "__main__": | ||
| try: | ||
| target = float(input(f"Enter target height in inches ({LOWEST_HEIGHT}-{HIGHEST_HEIGHT}): ")) |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent inch notation throughout the function: Lines 28, 30, 40, 46, and 50 use different notations for inches (double single-quotes '', no notation, or inconsistent formatting). Standardize to use the proper inch symbol (") throughout for clarity and consistency.
scripts/move_to_height.py
Outdated
| # Step 1: Reset to lowest | ||
| print(f"Resetting to lowest position ({LOWEST_HEIGHT})...") | ||
| release_down() | ||
| input("Press Enter to move all the way DOWN (about 56 seconds from max height)...") |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading prompt message: The message says "about 56 seconds from max height" but this function is called during reset which should start from the current position (unknown), not necessarily from max height. This could confuse users if the desk isn't at max height.
| input("Press Enter to move all the way DOWN (about 56 seconds from max height)...") | |
| input("Press Enter to move all the way DOWN (this will take up to 56 seconds)...") |
scripts/move_to_height.py
Outdated
| release_down() | ||
| input("Press Enter to move all the way DOWN (about 56 seconds from max height)...") | ||
| press_down() | ||
| time.sleep(56.0) |
Copilot
AI
Nov 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded magic number: The 56.0 second duration is duplicated from reset_to_lowest.py without explanation. Consider extracting this as a named constant (e.g., MAX_TO_MIN_TIME = 56.0) with a comment explaining its derivation, or better yet, calculate it from (HIGHEST_HEIGHT - LOWEST_HEIGHT) / DOWN_RATE where DOWN_RATE = 0.55.
…working OK. Still assumes developer permissions.
…for desk lifter control
…ctor GPIO pin management
|
See requirements below:
Related to last point, including a snippet from context7 chat below: |
- Updated duty_cycle.py to enhance movement validation against duty cycle limits. - Introduced generate_movements.py for generating movement configurations based on current duty cycle status. - Modified lifter_state.json to reflect updated usage periods and total up time. - Enhanced main.py to support new commands for deploying immediate and custom movements. - Created movement_configs.json to store generated movement configurations. - Updated prefect_flows.py to simplify flow definitions and integrate custom movements execution. - Added duty cycle monitoring flow for scheduled checks and recommendations based on capacity.
No description provided.